gh-81371: implementing checking async generator methods#102129
gh-81371: implementing checking async generator methods#102129wrongnull wants to merge 3 commits intopython:mainfrom
Conversation
This PR may fix python#81371
|
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
well, now I'm not sure if this changes matches your excpectations about the way how stdlib should look like
|
Is there one certain way to define if callable returns awaitable? I mean it is possible to reimplement some logic in C. For instance callable can has special flag to this purpose. It's because I'm not sure if my patch handles all possible corner cases so maybe there are more subtle ones. So if the checked object was not declared with async def the only way to check this condition is brute force it manually, I guess. |
sobolevn
left a comment
There was a problem hiding this comment.
Couple of thoughts:
- There are other ways to do that. Flags, coroutine marks, etc. Why this one is the best one? (I am not an asyncio expert)
- Is it a problem worth fixing at all?
| @@ -0,0 +1 @@ | |||
| This PR may fix the issue 81371 | |||
There was a problem hiding this comment.
May? Or does it? Please, also specify what the issue is. These are user-facing notes. It must be clear without a github link.
There was a problem hiding this comment.
May? Or does it? Please, also specify what the issue is. These are user-facing notes. It must be clear without a github link.
I'm not 100% sure if it does. In the original issue those methods just returns some awaitable objects that doesn't pass the check. And I fixed it, but only for this certain example (I will add the tests soon). The obstacle is that I don't know if there are similar functions somewhere in the inerpreter. I'm not an internals expert. It would be great if you or someone else could suggest me such places.
| Coroutine functions are normally defined with "async def" syntax, but may | ||
| be marked via markcoroutinefunction. | ||
| """ | ||
| #check if obj is async gen method and returns awaitable |
There was a problem hiding this comment.
Please, at least add tests for this feature.
There was a problem hiding this comment.
as soon as I fully understand the original question
See #81371 (comment) I don't think the current approach in this PR is the right approach; we shouldn't be checking against an approved list of method names. |
But there is no way to add attribute to builtin function without monkey patching. Do we need new type(s) to this? Such as |
|
What if |
|
This isn't the way to fix this, I am closing this, please continue the discussion on the issue. |
This PR may fix #81371